Support embedded go_proto_library #5567

tingilee · 2023-10-30T19:13:02Z

Checklist

I have filed an issue about this change and discussed potential changes with the maintainers.
I have received the approval from the maintainers to make this change.
This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #5566

Description of this change

Support embedded go_proto_library as source files in the same Go package.

With this change, proto refers to the pb.go as source files which is what we intend to do:

cat bazel-bin/proto/proto-106940904.intellij-info.txt
build_file_artifact_location {
  is_external: false
  is_source: true
  relative_path: "proto/BUILD.bazel"
  root_execution_path_fragment: ""
}
deps {
  dependency_type: 0
  target {
    label: "//proto:fooproto_go_proto"
  }
}
go_ide_info {
  import_path: "github.com/bazelbuild/intellij/examples/go/with_proto/proto"
  sources {
    is_external: false
    is_source: true
    relative_path: "proto/translators.go"
    root_execution_path_fragment: ""
  }
  sources {
    is_external: false
    is_source: false
    relative_path: "proto/fooproto_go_proto_/github.com/bazelbuild/intellij/examples/go/with_proto/proto/fooserver.pb.go"
    root_execution_path_fragment: "bazel-out/darwin_arm64-fastbuild/bin"
  }
}
key {
  label: "//proto:proto"
}
kind_string: "go_library"

agluszak · 2023-11-06T11:06:53Z

Hi! Thanks @tingilee for your contribution. Could you please write a test for this?

tingilee · 2023-11-09T07:29:43Z

WORKSPACE

-    sha256 = "825c30d2cbcce405fd18fddf356eb1f425607e9c780f8eff95d21ac23f8d90fd",
-    url = "https://plugins.jetbrains.com/maven/com/jetbrains/plugins/PythonCore/231.8770.65/PythonCore-231.8770.65.zip",
-)
-


This rule seems redundant so deleting here. I had to delete this otherwise I get errors:

Error in fail: generate_repo_config: generate_repo_config: /workdir/WORKSPACE: multiple rules have the name "python_2023_1"

See CI build failure: https://buildkite.com/bazel/intellij-plugin-aspect/builds/20467

Here are the duplicated rules:

intellij/WORKSPACE

Lines 320 to 336 in ae7424b

http_archive(

name = "python_2023_1",

build_file_content = _PYTHON_CE_BUILD_FILE,

sha256 = PYTHON_PLUGIN_231_SHA,

url = PYTHON_PLUGIN_231_URL,

)

PYTHON_PLUGIN_232_URL = "https://plugins.jetbrains.com/maven/com/jetbrains/plugins/PythonCore/232.10203.2/PythonCore-232.10203.2.zip"

PYTHON_PLUGIN_232_SHA = "027bc9e2322f21cb3093b3254035cfeaac587552f9db2f664b28280f172acfed"

http_archive(

name = "python_2023_2",

build_file_content = _PYTHON_CE_BUILD_FILE,

sha256 = PYTHON_PLUGIN_232_SHA,

url = PYTHON_PLUGIN_232_URL,

)

tingilee · 2023-11-09T07:30:28Z

Hi! Thanks @tingilee for your contribution. Could you please write a test for this?

Added tests and also include dependencies needed in the WORKSPACE.

agluszak · 2023-11-10T14:25:46Z

LGTM, but there's a conflict.

@tpasternak want to have a final look?
@mai93 what's the policy for introducing new dependencies to WORKSPACE?

tpasternak · 2023-11-13T14:00:27Z

@tpasternak want to have a final look?
So I just don't fully understand why it is required to add all the dependencies to the WORKSPACE file. How do we use them?

tingilee · 2023-11-14T01:23:52Z

@tpasternak

So I just don't fully understand why it is required to add all the dependencies to the WORKSPACE file. How do we use them?

These dependencies where added similarly to

intellij/WORKSPACE

Lines 865 to 881 in f49aaf2

    
           # Dependency needed for kotlin coroutines library 
        
           jvm_maven_import_external( 
        
               name = "kotlinx_coroutines", 
        
               artifact = "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.6.2", 
        
               artifact_sha256 = "09aac136027678db2d3c2696696202719af9213ba17ae076f4c4421008885bcb", 
        
               licenses = ["notice"],  # Apache 2.0 
        
               server_urls = ["https://repo1.maven.org/maven2"], 
        
           ) 
        
           # Dependency needed for kotlin coroutines test library 
        
           jvm_maven_import_external( 
        
               name = "kotlinx_coroutines_test", 
        
               artifact = "org.jetbrains.kotlinx:kotlinx-coroutines-test-jvm:1.6.2", 
        
               artifact_sha256 = "6eb5c29f60fcacde882b1d393bf9a2fe9535bece1c707396fdbd755559dc043d", 
        
               licenses = ["notice"],  # Apache 2.0 
        
               server_urls = ["https://repo1.maven.org/maven2"], 
        
           )

. Previously, there was no tests for Bazel + Go integration in IntelliJ. To address the PR comments, I've added tests for Bazel + Go integrations, which means we need the golang dependencies, specifically all of the dependencies to support aspect/testing/tests/src/com/google/idea/blaze/aspect/go/go_proto_library/BUILD

blorente

@tingilee I think what @tpasternak meant was that we don't reference those dependencies anywhere in the BUILD files, so it's unclear how they are needed for the tests. For instance, I'd expect to see @org_golang_x_net somewhere in the test build file.

blorente · 2023-11-15T13:50:18Z

aspect/testing/tests/src/com/google/idea/blaze/aspect/go/go_proto_library/setup.bzl

@@ -0,0 +1,4 @@
+def setup_tests():


I'm not sure what this file is for, it doesn't seem to be referenced anywhere.

blorente · 2023-11-15T13:51:13Z

aspect/testing/tests/src/com/google/idea/blaze/aspect/go/go_proto_library/simple.go

@@ -0,0 +1 @@
+package simple


Same with this file, it doesn't seem to be referenced.

blorente · 2024-01-29T14:13:33Z

@tingilee I've rebased this PR and addressed the comments on this branch: https://github.com/blorente/intellij/tree/blorente/address-comments. Should be ready to merge.

If you're okay with it, feel free to rebase this PR on top of mine. Or I can just merge my version if you're busy, whatever works. This functionality would be very useful for us.

blorente · 2024-02-05T10:27:42Z

@tpasternak @mai93 , would you be okay if I open a PR with the cleanups and we merge this? Some of our users really liked the feature, and it seems generally useful.

tpasternak · 2024-02-05T11:23:17Z

@tpasternak @mai93 , would you be okay if I open a PR with the cleanups and we merge this? Some of our users really liked the feature, and it seems generally useful.

Why would I have anything against it? 😅 Are there any disadvantages?

blorente · 2024-02-06T09:58:12Z

@tingilee I opened a PR with the cleanup here: #6030
Please let us know in a couple of days if you'd like to incorporate the changes to yours, otherwise we'll merge it.

ramilmsh · 2024-02-06T10:08:58Z

@blorente Hi!

I've tried out our plugin version and unfortunately it doesn't seem to solve

#5538

Specifically, If an embedded proto library depends on an embedded proto library, resolution still fails. I have not yet had the time to create a more complex use-case in my demo repo, which I will try to do this week, but I'd be happy to demonstrate the problem. I'm also not against contributing a fix myself, but I have no idea how intellij works and how the plugin is structured, so it would take me really-really long to do so.

blorente · 2024-02-12T10:48:30Z

@ramilmsh I've tried to reproduce the case you mention in https://github.com/blorente/intellij/blob/blorente/repro-oss-issue-with-proto-in-proto/examples/go/with_proto/proto/BUILD.bazel, and haven't been able to.
Since this PR, though it may be incomplete, is useful on its own, I'd like to merge it soon. Let's hop over to the issue you opened to discuss if it's missing some features, and how to implement them.

blorente · 2024-03-19T17:17:58Z

Closing since #6030 merged.

tingilee requested review from mai93, jastice, tpasternak and agluszak as code owners October 30, 2023 19:13

github-actions bot added product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Oct 30, 2023

github-actions bot assigned blorente Oct 30, 2023

tingilee mentioned this pull request Oct 30, 2023

Embedded go_proto_library not being indexed #5566

Closed

iancha1992 linked an issue Oct 30, 2023 that may be closed by this pull request

Embedded go_proto_library not being indexed #5566

Closed

tingilee force-pushed the jacqueline-go-proto-library branch from 951d739 to 115d9ba Compare November 9, 2023 07:19

tingilee commented Nov 9, 2023

View reviewed changes

agluszak self-assigned this Nov 10, 2023

jacqueline.lee and others added 3 commits November 10, 2023 08:22

Support embedded go_proto_library

fe1c61a

[aspect] Add tests for go_proto_library

d4f4652

Delete duplicated rules in WORKSPACE

f49aaf2

tingilee force-pushed the jacqueline-go-proto-library branch from 4d776f2 to f49aaf2 Compare November 10, 2023 16:24

blorente reviewed Nov 15, 2023

View reviewed changes

blorente mentioned this pull request Feb 6, 2024

(rebase) Support embedded go_proto_library #6030

Merged

blorente mentioned this pull request Feb 12, 2024

Resolution fails for go files inline with proto files #5538

Closed

blorente closed this Mar 19, 2024

github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Mar 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support embedded go_proto_library #5567

Support embedded go_proto_library #5567

tingilee commented Oct 30, 2023 •

edited

Loading

agluszak commented Nov 6, 2023

tingilee Nov 9, 2023 •

edited

Loading

tingilee commented Nov 9, 2023

agluszak commented Nov 10, 2023

tpasternak commented Nov 13, 2023

tingilee commented Nov 14, 2023 •

edited

Loading

blorente left a comment

blorente Nov 15, 2023

blorente Nov 15, 2023

blorente commented Jan 29, 2024

blorente commented Feb 5, 2024

tpasternak commented Feb 5, 2024

blorente commented Feb 6, 2024

ramilmsh commented Feb 6, 2024

blorente commented Feb 12, 2024

blorente commented Mar 19, 2024

	http_archive(
	name = "python_2023_1",
	build_file_content = _PYTHON_CE_BUILD_FILE,
	sha256 = PYTHON_PLUGIN_231_SHA,
	url = PYTHON_PLUGIN_231_URL,
	)

	PYTHON_PLUGIN_232_URL = "https://plugins.jetbrains.com/maven/com/jetbrains/plugins/PythonCore/232.10203.2/PythonCore-232.10203.2.zip"

	PYTHON_PLUGIN_232_SHA = "027bc9e2322f21cb3093b3254035cfeaac587552f9db2f664b28280f172acfed"

	http_archive(
	name = "python_2023_2",
	build_file_content = _PYTHON_CE_BUILD_FILE,
	sha256 = PYTHON_PLUGIN_232_SHA,
	url = PYTHON_PLUGIN_232_URL,
	)

Support embedded go_proto_library #5567

Support embedded go_proto_library #5567

Conversation

tingilee commented Oct 30, 2023 • edited Loading

Checklist

Discussion thread for this change

Description of this change

agluszak commented Nov 6, 2023

tingilee Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

tingilee commented Nov 9, 2023

agluszak commented Nov 10, 2023

tpasternak commented Nov 13, 2023

tingilee commented Nov 14, 2023 • edited Loading

blorente left a comment

Choose a reason for hiding this comment

blorente Nov 15, 2023

Choose a reason for hiding this comment

blorente Nov 15, 2023

Choose a reason for hiding this comment

blorente commented Jan 29, 2024

blorente commented Feb 5, 2024

tpasternak commented Feb 5, 2024

blorente commented Feb 6, 2024

ramilmsh commented Feb 6, 2024

blorente commented Feb 12, 2024

blorente commented Mar 19, 2024

tingilee commented Oct 30, 2023 •

edited

Loading

tingilee Nov 9, 2023 •

edited

Loading

tingilee commented Nov 14, 2023 •

edited

Loading